Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added adjust_brightness and adjust_contrast to transforms.py #15046 #18363

Closed
wants to merge 1 commit into from

Conversation

strauhal
Copy link

@strauhal strauhal commented Jul 5, 2023

#15046

Will add more to transforms.py if my formatting is alright.

@ivy-leaves ivy-leaves added the Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist label Jul 5, 2023
@ivy-leaves
Copy link

If you are working on an open task, please edit the PR description to link to the issue you've created.

For more information, please check ToDo List Issues Guide.

Thank you 🤗

@hmahmood24
Copy link
Contributor

Hey @strauhal, thanks for the PR. However, looking more closely, it’s unclear how this PR would meaningfully contribute towards the project, because of the following points:

1.You haven't linked the sub-task issue(s) you are working on by commenting Close #<issue_number> on this PR. Also, there seems to be 2 functions added so ideally you should create 2 issues and correspondingly 2 PRs linked to the issues. One for each of the function.
2. The PR doesn't follow any of the formats we support for adding frontend functions and it lacks the tests for these frontend functions as well.
3. The PR lacks the lint/formatting standards that we need to stick to. You can use pre-commit to run the lint checks before commiting so your PR auto-formats to align with the standards we need to adhere to.
4. We don't use third-party/native libraries in the frontends e.g. PIL or numpy so you'll need to change your implementation to use ivy functions only.
5. You haven't applied the necessary function decorators that we need to apply to all of our frontend functions e.g. to_ivy_arrays_and_back etc.
6. You haven't added any test for your added functions.

With these points in mind, I’ve simply closed the PR for now 🙂

Please feel free to submit other PRs based on our Open Tasks after going through our Contributing and Deep Dive guides first.

Thanks!

@hmahmood24 hmahmood24 closed this Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants